core: crypto: libtomcrypt: fix LTC_CLEAN_STACK bug #3102
Conversation
|
As I said previously [1] this way of scrubbing the stack is wrong IMO, but the code introduced by this PR is clearly much better since it is much less likely to cause a stack overflow, as per your analysis in [2]. With the commit text clarified:
[1] #3064 (comment) |
Agree,
Sure, will clarify commit text. |
LTC_CLEAN_STACK uses burn_stack() API that uses a recursive call which leads to approx. double the size of stack cleaned than expected on ARM64, because it consumes stack space in 32-byte chunks and assumes only buf is pushed onto the stack while ignoring any other data such as lr, fp, etc.. This causes stack overflow corrupting canaries in case we perform a SHA512 hash operation which utilizes maximum stack as compared to other libtomcrypt APIs. So get rid of this recursive call via using variable length array to clean stack. Also, convert zeromem() API as a wrapper to call memzero_explicit(). Fixes: ad56511 ("core: crypto: libtomcrypt: enable LTC_CLEAN_STACK") Suggested-by: Daniel Thompson <[email protected]> Signed-off-by: Sumit Garg <[email protected]> Acked-by: Jerome Forissier <[email protected]>
|
Updated PR. |
Implements a cross-platform version of OP-TEE/optee_os#3102 1. Remove recursion 2. Use memset instead of while loop
Fixing status-im/status-desktop#10572 Implements a cross-platform version of OP-TEE/optee_os#3102 Remove recursion Use memset instead of while loop A description to understand introduced changes without reading the code. zeromem weights about 50% of the total CPU time on M1 Macs and seems to be major performance offender. It is used to clear the stack when using variables with sensitive information.
Implements a cross-platform version of OP-TEE/optee_os#3102 1. Remove recursion 2. Use memset instead of while loop
Implements a cross-platform version of OP-TEE/optee_os#3102 1. Remove recursion 2. Use memset instead of while loop
LTC_CLEAN_STACK uses burn_stack() API that uses a recursive call which
leads to approx. double the size of stack cleaned than expected on ARM64.
So this causes stack overflow corrupting canaries in case we perform a
SHA512 hash operation which utilizes maximum stack as compared to other
libtomcrypt APIs. So get rid of this recursive call via using variable
length array to clean stack.
Also, convert zeromem() API as a wrapper to call memzero_explicit().
Fixes: ad56511 ("core: crypto: libtomcrypt: enable LTC_CLEAN_STACK")
Suggested-by: Daniel Thompson [email protected]
Signed-off-by: Sumit Garg [email protected]